Skip to content

Implement P2248R8: Enable list-initialization for algorithms #2398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
rarutyun merged 9 commits into from
Dec 5, 2025
Merged

Implement P2248R8: Enable list-initialization for algorithms #2398

rarutyun merged 9 commits into from
Dec 5, 2025

Conversation

@rarutyun
Copy link
Contributor

@rarutyun rarutyun commented Aug 25, 2025 •
edited
Loading

The link to the proposal

Implementing Iterator-based algorithms only. Ranges-based implementation is going on separately.

There are no breaking changes. All the breaking changes in C++ standard come into std::ranges namespace but even they are irrelevant to oneDPL because we implement list-initialization enabling right away, so there is no template parameters swapping

There are two oversights for P2248:

  • find_last was not included (see this paper). However, it's irrelevant for Iterator-based algorithms because find_last exists in std::ranges: namespace only
  • uninitialized_fill was not included (see this paper. However, it's not included for C++26 yet. I've implemented that proactively as was requested. There is a NB comment to include this oversight for C++26.

Binary Search-like algorithms from P2248 (e.g., lower_bound, upper_bound, etc.) are not relevant either because our API for those does not have T-like template parameter.

Linked issue: #1508

@rarutyun rarutyun self-assigned this Aug 25, 2025
@akukanov
Copy link
Contributor

akukanov commented Aug 27, 2025

  • uninitialized_fill was not included (see this paper. However, it's not included for C++26. We are trying to achieve that but there is no guarantee. The question is do we want to implement it proactively?

Yes. Let's do that for all algorithms, including - if necessary - non-standard ones in oneDPL.

@danhoeflinger
Copy link
Contributor

danhoeflinger commented Aug 28, 2025

Is the intention to merge this for milestone 2022.10.0?

@rarutyun
Copy link
Contributor Author

rarutyun commented Aug 28, 2025

Is the intention to merge this for milestone 2022.10.0?

Whenever we think it's ready

@rarutyun rarutyun mentioned this pull request Sep 3, 2025
Closed
@rarutyun rarutyun force-pushed the rarutyun/implement_p2248r8 branch from d0441d2 to f1b47c6 Compare October 24, 2025 18:43
@danhoeflinger danhoeflinger added this to the 2022.11.0 milestone Nov 17, 2025
danhoeflinger
danhoeflinger reviewed Nov 17, 2025
View reviewed changes
danhoeflinger
danhoeflinger reviewed Nov 17, 2025
View reviewed changes
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the question on replace_copy, I don't see any issues with the implementation, but I have not yet checked the tests.

danhoeflinger
danhoeflinger reviewed Nov 18, 2025
View reviewed changes
danhoeflinger
danhoeflinger reviewed Nov 21, 2025
View reviewed changes
@rarutyun
Implement P2248R8: Enable list-initialization for algorithms
9639f71
@rarutyun
Add default template argument tests. Improve replace_copy tests
bda6cde
@rarutyun
Implement empty list initialization for uninitialized_fill[_n]
008b5a8 
Tests for hetero backend remain
@rarutyun
Add tests for uninitialized_fill[_n] for hetero backend
b0c043f
@rarutyun
Address the review comments
a7c545f 
- remove the default template argument from replace_copy that was added
  by mistake
- remove runtime tests for replace_copy but keep compile-time ones
- remove redundant pair of curly braces where applicable
- improve the messages for static assert
@rarutyun rarutyun force-pushed the rarutyun/implement_p2248r8 branch from 205dc35 to a7c545f Compare December 2, 2025 21:20
danhoeflinger
danhoeflinger previously approved these changes Dec 2, 2025
View reviewed changes
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor wording issues only (and the clang-formatting suggestion).
If you prefer to keep "shall" over "must", that is OK.

LGTM, will reapprove after changes

danhoeflinger
danhoeflinger previously approved these changes Dec 3, 2025
View reviewed changes
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@danhoeflinger
Copy link
Contributor

danhoeflinger commented Dec 3, 2025

The fixes so far look good.

@rarutyun rarutyun force-pushed the rarutyun/implement_p2248r8 branch 2 times, most recently from 73c3ab2 to 1dc25f2 Compare December 4, 2025 04:10
@rarutyun rarutyun force-pushed the rarutyun/implement_p2248r8 branch from 1dc25f2 to c087a2d Compare December 4, 2025 04:32
@SergeyKopienko
Copy link
Contributor

SergeyKopienko commented Dec 4, 2025

Let me highlight the next moment.

As far as I understand this PR implement some changes according to the future C++ specifications.
But changed the API (at least formally) for C++ version which supported by oneDPL for now: 17 and 20.
This mean we allows to user to have something code with {} in the parameters for changed methods which will ill-formed with other standard library implementations for C++17 and C++20.

Is it OK ?

@akukanov
Copy link
Contributor

akukanov commented Dec 4, 2025 •
edited
Loading

This mean we allows to user to have something code with {} in the parameters for changed methods which will ill-formed with other standard library implementations for C++17 and C++20.

Correct. Users who prefer to keep their source code base compatible with the current and previous versions of the C++ standard should not use braced-list arguments for the standard algorithms. Even after C++26 is published and implemented, previous (already released) versions of the standard libraries won't support this ability. oneDPL will still support C++17/20/23, meeting (most of) the requirements for parallel algorithms in these standards. The change does not introduce incompatibilities there.

SergeyKopienko

This comment was marked as outdated.

Sign in to view

@SergeyKopienko SergeyKopienko self-requested a review December 4, 2025 18:24
SergeyKopienko
SergeyKopienko approved these changes Dec 4, 2025
View reviewed changes
Copy link
Contributor

@SergeyKopienko SergeyKopienko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rarutyun rarutyun merged commit cadae13 into main Dec 5, 2025
23 checks passed
@rarutyun rarutyun deleted the rarutyun/implement_p2248r8 branch December 5, 2025 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SergeyKopienko SergeyKopienko SergeyKopienko approved these changes

@danhoeflinger danhoeflinger danhoeflinger left review comments

@akukanov akukanov Awaiting requested review from akukanov

@dmitriy-sobolev dmitriy-sobolev Awaiting requested review from dmitriy-sobolev

@mmichel11 mmichel11 Awaiting requested review from mmichel11

@adamfidel adamfidel Awaiting requested review from adamfidel

Assignees

@rarutyun rarutyun

Labels

None yet

Projects

None yet

Milestone

2022.11.0

Development

Successfully merging this pull request may close these issues.

4 participants

@rarutyun @akukanov @danhoeflinger @SergeyKopienko